Skip to content

326 force nan replacement #327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

326 force nan replacement #327

wants to merge 7 commits into from

Conversation

@TonyB9000
Copy link
Contributor

@TonyB9000 TonyB9000 commented Oct 20, 2025 •
edited
Loading

Description

The v3.LR cases failed to support the CMIP6 variable `CFmon.clisccp" due to NaNs in the data where the standard FILL_Value was expected. Code is supplied in the cmor_handlers/vars/clisccp.py module that forces the replacement with the required FILL_Value, prior to calling cmor.write().

Checklist

  • [y] My code follows the style guidelines of this project
  • [y] I have performed a self-review of my own code
  • [?] My changes generate no new warnings
  • [na] Any dependent changes have been merged and published in downstream modules

If applicable:

  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

@tomvothecoder
Add --on-var-failure flag to handle failure behaviors
7b937ab 
- Defaults to ignoring failures
@tomvothecoder
Update sys.exit behavior based on derived handlers
258794a
@tomvothecoder
Update e3sm_to_cmip/cmor_handlers/utils.py
cbf2930
@tomvothecoder
Clean up logger messages and extract sys.exit funcs in util.py
990c172 
- Fix bug in `_get_handlers()` not instantianting `missing_handlers` after `_get_mpas_handlers()` call
- Add FIXME: comments for duplicate code
- Extract stop behaviors to `_stop_with_failed_handler()` and `_stop_with_failed_handler_parallel()`
@tomvothecoder
Fix unit tests & replace load_module()
153174e
@tomvothecoder
Apply suggestion from @tomvothecoder
638b1fe
@tomvothecoder
Adddress copilot review comments
bf9bf82
@tomvothecoder
Copy link
Collaborator

tomvothecoder commented Oct 20, 2025

@TonyB9000 You accidentally branched from my branch in PR #323. Can you instead branch from main and add your changes, then re-create this PR.

Alternatively, you can run git rebase on the latest commit of main and drop all commits related to #323 (the more complicated way if you're not familiar with this process).

@tomvothecoder
Copy link
Collaborator

tomvothecoder commented Oct 20, 2025

I actually don't see your np.nan fix in this branch either. I suggest just re-creating the branch from main with your np.nan fix, then re-creating this PR.

@TonyB9000 TonyB9000 closed this Oct 20, 2025
@TonyB9000 TonyB9000 deleted the 326-force-NAN-replacement branch October 20, 2025 21:00
@github-project-automation github-project-automation bot moved this from In progress to Done in E3SM to CMIP Development Oct 20, 2025
@TonyB9000
Copy link
Contributor Author

TonyB9000 commented Oct 20, 2025 •
edited
Loading

@tomvothecoder So ... I switched to my local "master" branch, then deleted my local branch 326-force-NAN-replacement. Then I did a "git pull" to obtain the master changes:

From https://github.com/E3SM-Project/e3sm_to_cmip
   f90e4ab..7c7f400  master            -> origin/master
 * [new branch]      bug/324-data-vars -> origin/bug/324-data-vars
Updating 19e3ee8..7c7f400
Fast-forward
 e3sm_to_cmip/__init__.py                    |  13 ++++++++++++-
 e3sm_to_cmip/cmor_handlers/handler.py       |   2 ++
 e3sm_to_cmip/cmor_handlers/handlers.yaml    |  48 ++++++++++++++++++++++++++++++++++++++++++++++++
 e3sm_to_cmip/cmor_handlers/mpas_vars/pbo.py |   9 +++++++--
 e3sm_to_cmip/cmor_handlers/mpas_vars/pso.py |   9 +++++++--
 e3sm_to_cmip/cmor_handlers/vars/clmodis.py  | 178 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 e3sm_to_cmip/mpas.py                        |   8 +++++++-
 e3sm_to_cmip/runner.py                      |   2 +-
 tbump.toml                                  |   2 +-
 9 files changed, 263 insertions(+), 8 deletions(-)
 create mode 100644 e3sm_to_cmip/cmor_handlers/vars/clmodis.py

Then I did git checkout -b 326-force-NAN-replacement (switched to new branch).

I went to edit "cmor_handlers/vars/clisccp.py", and found that my changes were still in place.

SO, I did the git add and git commit -m message.

When I tried: git push --set-upstream origin 326-force-NAN-replacement, I got

To https://github.com/E3SM-Project/e3sm_to_cmip
 ! [rejected]        326-force-NAN-replacement -> 326-force-NAN-replacement (non-fast-forward)
error: failed to push some refs to 'https://github.com/E3SM-Project/e3sm_to_cmip'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. If you want to integrate the remote changes,
hint: use 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

So, the problem is I deleted my local "new branch" but not its "remote counterpart"

Next: git push origin --delete 326-force-NAN-replacement

and git push --set-upstream origin 326-force-NAN-replacement

SUCCESS

@TonyB9000
Copy link
Contributor Author

TonyB9000 commented Oct 20, 2025

Reopened after deleting the branch and branching from main (git pulled to bring up-to-date).

@TonyB9000
Copy link
Contributor Author

TonyB9000 commented Oct 20, 2025

@tomvothecoder Am I close? I assume re-opening resumes the PR.

@tomvothecoder
Copy link
Collaborator

tomvothecoder commented Oct 20, 2025

@TonyB9000 Now you need to open a new PR with the fixed branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomvothecoder tomvothecoder Awaiting requested review from tomvothecoder

Assignees

No one assigned

Labels

None yet

Projects

E3SM to CMIP Development
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: failure to cmorize v3.LR variable CFmon.clisccp due to NaNs in data

3 participants

@TonyB9000 @tomvothecoder